fix: ignore case when matching function name #16912
Conversation
notauserx
changed the title
|
Does it make sense to add ignore case search in is_builtin_function(func_name) as well? |
| @@ -721,9 +721,7 @@ impl<'a> TypeChecker<'a> { | |||
| } => { | |||
| let func_name = normalize_identifier(name, self.name_resolution_ctx).to_string(); | |||
| let func_name = func_name.as_str(); | |||
There was a problem hiding this comment.
what about handling case here?
| let func_name = func_name.as_str(); | |
| let func_name = func_name.to_lowercase(); |
There was a problem hiding this comment.
Good suggestion.
Since sugar and built-in functions are matched ignoring case, that's why I created a sep function and moved the logic inside there, and we could do the same in is_builtin_function, otherwise each caller has to convert to lower case before calling the functions
There was a problem hiding this comment.
Ok. I see. Then I'll suggest replacing &str in all_sugar_functions and builtin_functions to https://crates.io/crates/unicase
There was a problem hiding this comment.
I've used the Ascii type from the unicase crate, a case-insensitive wrapper for ASCII strings. Is this assumption correct? If not, I can switch to the UniCase type, which provides a case-insensitive wrapper for general strings.
|
Let's add a test in |
notauserx
force-pushed
the
fix-search-by-lowercase-func-name
branch
2 times, most recently
from
f64459f to
5ad40f7
Compare
notauserx
force-pushed
the
fix-search-by-lowercase-func-name
branch
from
c08754d to
d4bd725
Compare
Added a test |
I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/
Summary
Update logic to search by the lowercase of func_name. Uses unicode crate to facilitate searching function name ignoring case
fixes: #16737
Tests
Type of change
This change is